I wonder if the Ping() would be better placed in grabConn() (agent-side) instead of ConnHandler() (server-side). Adding a goroutine per active session on the server doesn't scale. On the agent side each process only manages its own sessions, so the cost is distributed.

gorilla/websocket already responds to pings with pongs during NextReader(). No extra goroutine on the server, same bidirectional traffic for the LB.

// pkg/revdial/revdial.go — grabConn(), line 442
c := wsconnadapter.New(wsConn)
c.Ping()

select {
case ln.connc <- c:
case <-ln.donec:
}

@gustavosbarreto Good call — I've moved Ping() to grabConn() on the agent side as you suggested. This distributes the goroutine cost across agents instead of concentrating it on the server.

This fix works for the immediate issue, but we'll need to think more deeply about how we want to implement this long-term. Given that we've never had agent integration in other languages, we need to find a way to enable this back-and-forth communication that accounts for all clients. Some of our customers rely on a large number of devices, and in those cases this extra WebSocket ping/pong transmission on every active session could impact performance.

Sharing some research on cloud load balancer behavior that reinforces why this keepalive fix is necessary.

All major cloud vendors enforce idle timeouts on load-balanced TCP/WebSocket connections, and some require application-level data (not just TCP keepalives) to reset the timer:

AWS ALB and GCP TCP Proxy both require actual data in the payload to keep the connection alive — TCP-level keepalive packets aren't enough. WebSocket ping/pong frames count as application-level data, which is why our fix in #5947 works.

Ref: GCE discussion on TCP proxy LB timeouts

@gustavosbarreto — requesting your review on this one. The changes go beyond the original per-session keepalive fix and touch shared infrastructure, so I want to make sure nothing has unintended side effects.

What to pay attention to

1. wsconnadapter.Close() is now idempotent

The old Close() called conn.Close() on the underlying gorilla/websocket connection every time it was invoked. The new version uses sync.Once — only the first call actually closes the connection; subsequent calls return nil.

This matters because several code paths can trigger Close() concurrently or multiple times:

• The pong timeout time.AfterFunc fires and calls Close()
• The revdial Dialer.close() calls conn.Close()
• yamux session teardown calls Close() on the underlying connection

If any code path currently depends on getting an error back from a second Close() call (to detect "already closed"), it will now silently get nil. I reviewed the callers and didn't find such a case, but you know the platform better.

2. wsconnadapter.Ping() initialization is now atomic

The old nil-check guard (if a.pongCh != nil) was racy under concurrent calls. The new sync.Once ensures channels and the ping goroutine are created exactly once. Current callers only call Ping() once per adapter, so this is defensive — but worth verifying no future or cloud-specific code path calls it differently.

3. Gateway Nginx proxy_read_timeout 0 on all WebSocket locations

This disables Nginx's read timeout for /ssh/connection, /ssh/revdial, /agent/connection, and /ws. Application-level keepalive (30s ping/pong + JSON keep-alive) handles liveness instead.

The concern here is zombie connections: if the application-level keepalive ever stops working (bug, deadlock), Nginx won't clean up the connection — it will hang indefinitely. Previously, the 60s default acted as a safety net (albeit a fragile one). Worth considering whether we want a large finite value (e.g., proxy_read_timeout 3600) instead of 0 as a last-resort safety net.

@gustavosbarreto — the branch has been updated and all CI checks are now passing (unit tests + integration tests on both mongo and postgres).

The main change since your earlier review: the proxy_read_timeout 0 nginx change has been removed. During CI bisection we discovered that nginx interprets 0 as "zero seconds" (immediate timeout), not "infinite" — it was breaking all connections through the gateway. The application-level 30s ping/pong keepalive is sufficient to stay within nginx's default 60s timeout.

The remaining changes are:

1. Per-session WebSocket ping/pong in grabConn() (agent-side, per your suggestion)
2. Close() with sync.Once — idempotent, returns stored error on subsequent calls
3. Ping() with sync.Once — atomic initialization
4. Terminal resize test timeout increase (test flakiness fix)

Could you take another look when you get a chance?